| Message ID | 20210906020100.14430-15-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 06/09/2021 04:00, Laurent Pinchart wrote: > When dequeueing a buffer from a V4L2VideoDevice, the number of planes in > the FrameBuffer may not match the number of V4L2 buffer planes if the > PixelFormat is multi-planar (has multiple colour planes) and the V4L2 > format is single-planar (has a single buffer plane). In this case, we > need to split the single V4L2 buffer plane into FrameBuffer planes. Do > so, and add checks to reject invalid V4L2 buffers in case of a driver > issue. > What checks are those exactly ? The test on the dequeued buffer size ? > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Changes since v1: > > - Reduce indentation > --- > src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- > 1 file changed, 48 insertions(+), 5 deletions(-) > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 625d5da40337..0a7fbdb7e011 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -7,6 +7,7 @@ > > #include "libcamera/internal/v4l2_videodevice.h" > > +#include <algorithm> > #include <array> > #include <fcntl.h> > #include <iomanip> > @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() > buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL > + buf.timestamp.tv_usec * 1000ULL; > > - buffer->metadata_.planes.clear(); > - if (multiPlanar) { > - for (unsigned int nplane = 0; nplane < buf.length; nplane++) > - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); > + if (V4L2_TYPE_IS_OUTPUT(buf.type)) > + return buffer; > + > + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; > + FrameMetadata &metadata = buffer->metadata_; > + metadata.planes.clear(); > + > + if (numV4l2Planes != buffer->planes().size()) { > + /* > + * If we have a multi-planar buffer with a V4L2 > + * single-planar format, split the V4L2 buffer across > + * the buffer planes. Only the last plane may have less > + * bytes used than its length. > + */ > + if (numV4l2Planes != 1) { > + LOG(V4L2, Error) > + << "Invalid number of planes (" << numV4l2Planes > + << " != " << buffer->planes().size() << ")"; > + > + metadata.status = FrameMetadata::FrameError; > + return buffer; > + } > + > + unsigned int bytesused = multiPlanar ? planes[0].bytesused > + : buf.bytesused; > + > + for (auto [i, plane] : utils::enumerate(buffer->planes())) { > + if (!bytesused) { > + LOG(V4L2, Error) > + << "Dequeued buffer is too small"; > + > + metadata.status = FrameMetadata::FrameError; > + return buffer; > + } > + > + metadata.planes.push_back({ std::min(plane.length, bytesused) }); > + bytesused -= metadata.planes.back().bytesused; > + } > + } else if (multiPlanar) { > + /* > + * If we use the multi-planar API, fill in the planes. > + * The number of planes in the frame buffer and in the > + * V4L2 buffer is guaranteed to be equal at this point. > + */ > + for (unsigned int i = 0; i < numV4l2Planes; ++i) > + metadata.planes.push_back({ planes[i].bytesused }); > } else { > - buffer->metadata_.planes.push_back({ buf.bytesused }); > + metadata.planes.push_back({ buf.bytesused }); > } > > return buffer; >
Hi Jean-Michel, On Mon, Sep 06, 2021 at 09:22:56AM +0200, Jean-Michel Hautbois wrote: > On 06/09/2021 04:00, Laurent Pinchart wrote: > > When dequeueing a buffer from a V4L2VideoDevice, the number of planes in > > the FrameBuffer may not match the number of V4L2 buffer planes if the > > PixelFormat is multi-planar (has multiple colour planes) and the V4L2 > > format is single-planar (has a single buffer plane). In this case, we > > need to split the single V4L2 buffer plane into FrameBuffer planes. Do > > so, and add checks to reject invalid V4L2 buffers in case of a driver > > issue. > > What checks are those exactly ? The test on the dequeued buffer size ? Yes, and the test on the number of V4L2 planes. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Changes since v1: > > > > - Reduce indentation > > --- > > src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- > > 1 file changed, 48 insertions(+), 5 deletions(-) > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index 625d5da40337..0a7fbdb7e011 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -7,6 +7,7 @@ > > > > #include "libcamera/internal/v4l2_videodevice.h" > > > > +#include <algorithm> > > #include <array> > > #include <fcntl.h> > > #include <iomanip> > > @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() > > buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL > > + buf.timestamp.tv_usec * 1000ULL; > > > > - buffer->metadata_.planes.clear(); > > - if (multiPlanar) { > > - for (unsigned int nplane = 0; nplane < buf.length; nplane++) > > - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); > > + if (V4L2_TYPE_IS_OUTPUT(buf.type)) > > + return buffer; > > + > > + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; > > + FrameMetadata &metadata = buffer->metadata_; > > + metadata.planes.clear(); > > + > > + if (numV4l2Planes != buffer->planes().size()) { > > + /* > > + * If we have a multi-planar buffer with a V4L2 > > + * single-planar format, split the V4L2 buffer across > > + * the buffer planes. Only the last plane may have less > > + * bytes used than its length. > > + */ > > + if (numV4l2Planes != 1) { > > + LOG(V4L2, Error) > > + << "Invalid number of planes (" << numV4l2Planes > > + << " != " << buffer->planes().size() << ")"; > > + > > + metadata.status = FrameMetadata::FrameError; > > + return buffer; > > + } > > + > > + unsigned int bytesused = multiPlanar ? planes[0].bytesused > > + : buf.bytesused; > > + > > + for (auto [i, plane] : utils::enumerate(buffer->planes())) { > > + if (!bytesused) { > > + LOG(V4L2, Error) > > + << "Dequeued buffer is too small"; > > + > > + metadata.status = FrameMetadata::FrameError; > > + return buffer; > > + } > > + > > + metadata.planes.push_back({ std::min(plane.length, bytesused) }); > > + bytesused -= metadata.planes.back().bytesused; > > + } > > + } else if (multiPlanar) { > > + /* > > + * If we use the multi-planar API, fill in the planes. > > + * The number of planes in the frame buffer and in the > > + * V4L2 buffer is guaranteed to be equal at this point. > > + */ > > + for (unsigned int i = 0; i < numV4l2Planes; ++i) > > + metadata.planes.push_back({ planes[i].bytesused }); > > } else { > > - buffer->metadata_.planes.push_back({ buf.bytesused }); > > + metadata.planes.push_back({ buf.bytesused }); > > } > > > > return buffer;
Hi Laurent, thank you for the patch. On Mon, Sep 6, 2021 at 8:58 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Jean-Michel, > > On Mon, Sep 06, 2021 at 09:22:56AM +0200, Jean-Michel Hautbois wrote: > > On 06/09/2021 04:00, Laurent Pinchart wrote: > > > When dequeueing a buffer from a V4L2VideoDevice, the number of planes in > > > the FrameBuffer may not match the number of V4L2 buffer planes if the > > > PixelFormat is multi-planar (has multiple colour planes) and the V4L2 > > > format is single-planar (has a single buffer plane). In this case, we > > > need to split the single V4L2 buffer plane into FrameBuffer planes. Do > > > so, and add checks to reject invalid V4L2 buffers in case of a driver > > > issue. > > > > What checks are those exactly ? The test on the dequeued buffer size ? > > Yes, and the test on the number of V4L2 planes. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > Changes since v1: > > > > > > - Reduce indentation > > > --- > > > src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- > > > 1 file changed, 48 insertions(+), 5 deletions(-) > > > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > > index 625d5da40337..0a7fbdb7e011 100644 > > > --- a/src/libcamera/v4l2_videodevice.cpp > > > +++ b/src/libcamera/v4l2_videodevice.cpp > > > @@ -7,6 +7,7 @@ > > > > > > #include "libcamera/internal/v4l2_videodevice.h" > > > > > > +#include <algorithm> > > > #include <array> > > > #include <fcntl.h> > > > #include <iomanip> > > > @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() > > > buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL > > > + buf.timestamp.tv_usec * 1000ULL; > > > > > > - buffer->metadata_.planes.clear(); > > > - if (multiPlanar) { > > > - for (unsigned int nplane = 0; nplane < buf.length; nplane++) > > > - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); > > > + if (V4L2_TYPE_IS_OUTPUT(buf.type)) > > > + return buffer; > > > + > > > + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; > > > + FrameMetadata &metadata = buffer->metadata_; > > > + metadata.planes.clear(); > > > + > > > + if (numV4l2Planes != buffer->planes().size()) { > > > + /* > > > + * If we have a multi-planar buffer with a V4L2 > > > + * single-planar format, split the V4L2 buffer across > > > + * the buffer planes. Only the last plane may have less > > > + * bytes used than its length. > > > + */ > > > + if (numV4l2Planes != 1) { > > > + LOG(V4L2, Error) > > > + << "Invalid number of planes (" << numV4l2Planes > > > + << " != " << buffer->planes().size() << ")"; > > > + > > > + metadata.status = FrameMetadata::FrameError; > > > + return buffer; > > > + } > > > + > > > + unsigned int bytesused = multiPlanar ? planes[0].bytesused > > > + : buf.bytesused; > > > + > > > + for (auto [i, plane] : utils::enumerate(buffer->planes())) { > > > + if (!bytesused) { > > > + LOG(V4L2, Error) > > > + << "Dequeued buffer is too small"; > > > + > > > + metadata.status = FrameMetadata::FrameError; > > > + return buffer; > > > + } > > > + > > > + metadata.planes.push_back({ std::min(plane.length, bytesused) }); > > > + bytesused -= metadata.planes.back().bytesused; > > > + } > > > + } else if (multiPlanar) { > > > + /* > > > + * If we use the multi-planar API, fill in the planes. > > > + * The number of planes in the frame buffer and in the > > > + * V4L2 buffer is guaranteed to be equal at this point. > > > + */ > > > + for (unsigned int i = 0; i < numV4l2Planes; ++i) > > > + metadata.planes.push_back({ planes[i].bytesused }); > > > } else { > > > - buffer->metadata_.planes.push_back({ buf.bytesused }); > > > + metadata.planes.push_back({ buf.bytesused }); > > > } I think the latter two cases can be just } else { for (unsigned int i = 0; i < numV4l2Planes; ++i) metadata.planes.push_back({ planes[i].bytesused }); } With the nit, Reviewed-by: Hirokazu Honda <hiroh@chromium.org> -Hiro > > > > > > return buffer; > > -- > Regards, > > Laurent Pinchart
Hi Hiro, On Mon, Sep 06, 2021 at 10:16:47PM +0900, Hirokazu Honda wrote: > On Mon, Sep 6, 2021 at 8:58 PM Laurent Pinchart wrote: > > On Mon, Sep 06, 2021 at 09:22:56AM +0200, Jean-Michel Hautbois wrote: > > > On 06/09/2021 04:00, Laurent Pinchart wrote: > > > > When dequeueing a buffer from a V4L2VideoDevice, the number of planes in > > > > the FrameBuffer may not match the number of V4L2 buffer planes if the > > > > PixelFormat is multi-planar (has multiple colour planes) and the V4L2 > > > > format is single-planar (has a single buffer plane). In this case, we > > > > need to split the single V4L2 buffer plane into FrameBuffer planes. Do > > > > so, and add checks to reject invalid V4L2 buffers in case of a driver > > > > issue. > > > > > > What checks are those exactly ? The test on the dequeued buffer size ? > > > > Yes, and the test on the number of V4L2 planes. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > Changes since v1: > > > > > > > > - Reduce indentation > > > > --- > > > > src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- > > > > 1 file changed, 48 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > > > index 625d5da40337..0a7fbdb7e011 100644 > > > > --- a/src/libcamera/v4l2_videodevice.cpp > > > > +++ b/src/libcamera/v4l2_videodevice.cpp > > > > @@ -7,6 +7,7 @@ > > > > > > > > #include "libcamera/internal/v4l2_videodevice.h" > > > > > > > > +#include <algorithm> > > > > #include <array> > > > > #include <fcntl.h> > > > > #include <iomanip> > > > > @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() > > > > buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL > > > > + buf.timestamp.tv_usec * 1000ULL; > > > > > > > > - buffer->metadata_.planes.clear(); > > > > - if (multiPlanar) { > > > > - for (unsigned int nplane = 0; nplane < buf.length; nplane++) > > > > - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); > > > > + if (V4L2_TYPE_IS_OUTPUT(buf.type)) > > > > + return buffer; > > > > + > > > > + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; > > > > + FrameMetadata &metadata = buffer->metadata_; > > > > + metadata.planes.clear(); > > > > + > > > > + if (numV4l2Planes != buffer->planes().size()) { > > > > + /* > > > > + * If we have a multi-planar buffer with a V4L2 > > > > + * single-planar format, split the V4L2 buffer across > > > > + * the buffer planes. Only the last plane may have less > > > > + * bytes used than its length. > > > > + */ > > > > + if (numV4l2Planes != 1) { > > > > + LOG(V4L2, Error) > > > > + << "Invalid number of planes (" << numV4l2Planes > > > > + << " != " << buffer->planes().size() << ")"; > > > > + > > > > + metadata.status = FrameMetadata::FrameError; > > > > + return buffer; > > > > + } > > > > + > > > > + unsigned int bytesused = multiPlanar ? planes[0].bytesused > > > > + : buf.bytesused; > > > > + > > > > + for (auto [i, plane] : utils::enumerate(buffer->planes())) { > > > > + if (!bytesused) { > > > > + LOG(V4L2, Error) > > > > + << "Dequeued buffer is too small"; > > > > + > > > > + metadata.status = FrameMetadata::FrameError; > > > > + return buffer; > > > > + } > > > > + > > > > + metadata.planes.push_back({ std::min(plane.length, bytesused) }); > > > > + bytesused -= metadata.planes.back().bytesused; > > > > + } > > > > + } else if (multiPlanar) { > > > > + /* > > > > + * If we use the multi-planar API, fill in the planes. > > > > + * The number of planes in the frame buffer and in the > > > > + * V4L2 buffer is guaranteed to be equal at this point. > > > > + */ > > > > + for (unsigned int i = 0; i < numV4l2Planes; ++i) > > > > + metadata.planes.push_back({ planes[i].bytesused }); > > > > } else { > > > > - buffer->metadata_.planes.push_back({ buf.bytesused }); > > > > + metadata.planes.push_back({ buf.bytesused }); > > > > } > > I think the latter two cases can be just > } else { > for (unsigned int i = 0; i < numV4l2Planes; ++i) > metadata.planes.push_back({ planes[i].bytesused }); > } In the multi planar case, the bytesused value is retrieved from planes[i].bytesused, while in the single planar case, it comes from buf.bytesused. > With the nit, > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > > > > > > > return buffer;
Hi Laurent, On Mon, Sep 6, 2021 at 10:39 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Hiro, > > On Mon, Sep 06, 2021 at 10:16:47PM +0900, Hirokazu Honda wrote: > > On Mon, Sep 6, 2021 at 8:58 PM Laurent Pinchart wrote: > > > On Mon, Sep 06, 2021 at 09:22:56AM +0200, Jean-Michel Hautbois wrote: > > > > On 06/09/2021 04:00, Laurent Pinchart wrote: > > > > > When dequeueing a buffer from a V4L2VideoDevice, the number of planes in > > > > > the FrameBuffer may not match the number of V4L2 buffer planes if the > > > > > PixelFormat is multi-planar (has multiple colour planes) and the V4L2 > > > > > format is single-planar (has a single buffer plane). In this case, we > > > > > need to split the single V4L2 buffer plane into FrameBuffer planes. Do > > > > > so, and add checks to reject invalid V4L2 buffers in case of a driver > > > > > issue. > > > > > > > > What checks are those exactly ? The test on the dequeued buffer size ? > > > > > > Yes, and the test on the number of V4L2 planes. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > --- > > > > > Changes since v1: > > > > > > > > > > - Reduce indentation > > > > > --- > > > > > src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- > > > > > 1 file changed, 48 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > > > > index 625d5da40337..0a7fbdb7e011 100644 > > > > > --- a/src/libcamera/v4l2_videodevice.cpp > > > > > +++ b/src/libcamera/v4l2_videodevice.cpp > > > > > @@ -7,6 +7,7 @@ > > > > > > > > > > #include "libcamera/internal/v4l2_videodevice.h" > > > > > > > > > > +#include <algorithm> > > > > > #include <array> > > > > > #include <fcntl.h> > > > > > #include <iomanip> > > > > > @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() > > > > > buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL > > > > > + buf.timestamp.tv_usec * 1000ULL; > > > > > > > > > > - buffer->metadata_.planes.clear(); > > > > > - if (multiPlanar) { > > > > > - for (unsigned int nplane = 0; nplane < buf.length; nplane++) > > > > > - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); > > > > > + if (V4L2_TYPE_IS_OUTPUT(buf.type)) > > > > > + return buffer; > > > > > + > > > > > + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; > > > > > + FrameMetadata &metadata = buffer->metadata_; > > > > > + metadata.planes.clear(); > > > > > + > > > > > + if (numV4l2Planes != buffer->planes().size()) { > > > > > + /* > > > > > + * If we have a multi-planar buffer with a V4L2 > > > > > + * single-planar format, split the V4L2 buffer across > > > > > + * the buffer planes. Only the last plane may have less > > > > > + * bytes used than its length. > > > > > + */ > > > > > + if (numV4l2Planes != 1) { > > > > > + LOG(V4L2, Error) > > > > > + << "Invalid number of planes (" << numV4l2Planes > > > > > + << " != " << buffer->planes().size() << ")"; > > > > > + > > > > > + metadata.status = FrameMetadata::FrameError; > > > > > + return buffer; > > > > > + } > > > > > + > > > > > + unsigned int bytesused = multiPlanar ? planes[0].bytesused > > > > > + : buf.bytesused; > > > > > + > > > > > + for (auto [i, plane] : utils::enumerate(buffer->planes())) { > > > > > + if (!bytesused) { > > > > > + LOG(V4L2, Error) > > > > > + << "Dequeued buffer is too small"; > > > > > + > > > > > + metadata.status = FrameMetadata::FrameError; > > > > > + return buffer; > > > > > + } > > > > > + > > > > > + metadata.planes.push_back({ std::min(plane.length, bytesused) }); > > > > > + bytesused -= metadata.planes.back().bytesused; > > > > > + } > > > > > + } else if (multiPlanar) { > > > > > + /* > > > > > + * If we use the multi-planar API, fill in the planes. > > > > > + * The number of planes in the frame buffer and in the > > > > > + * V4L2 buffer is guaranteed to be equal at this point. > > > > > + */ > > > > > + for (unsigned int i = 0; i < numV4l2Planes; ++i) > > > > > + metadata.planes.push_back({ planes[i].bytesused }); > > > > > } else { > > > > > - buffer->metadata_.planes.push_back({ buf.bytesused }); > > > > > + metadata.planes.push_back({ buf.bytesused }); > > > > > } > > > > I think the latter two cases can be just > > } else { > > for (unsigned int i = 0; i < numV4l2Planes; ++i) > > metadata.planes.push_back({ planes[i].bytesused }); > > } > > In the multi planar case, the bytesused value is retrieved from > planes[i].bytesused, while in the single planar case, it comes from > buf.bytesused. > Ah, right. Thanks. I missed it. > > With the nit, > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > > > > > > > > > > return buffer; > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 625d5da40337..0a7fbdb7e011 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -7,6 +7,7 @@ #include "libcamera/internal/v4l2_videodevice.h" +#include <algorithm> #include <array> #include <fcntl.h> #include <iomanip> @@ -1664,12 +1665,54 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL + buf.timestamp.tv_usec * 1000ULL; - buffer->metadata_.planes.clear(); - if (multiPlanar) { - for (unsigned int nplane = 0; nplane < buf.length; nplane++) - buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); + if (V4L2_TYPE_IS_OUTPUT(buf.type)) + return buffer; + + unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; + FrameMetadata &metadata = buffer->metadata_; + metadata.planes.clear(); + + if (numV4l2Planes != buffer->planes().size()) { + /* + * If we have a multi-planar buffer with a V4L2 + * single-planar format, split the V4L2 buffer across + * the buffer planes. Only the last plane may have less + * bytes used than its length. + */ + if (numV4l2Planes != 1) { + LOG(V4L2, Error) + << "Invalid number of planes (" << numV4l2Planes + << " != " << buffer->planes().size() << ")"; + + metadata.status = FrameMetadata::FrameError; + return buffer; + } + + unsigned int bytesused = multiPlanar ? planes[0].bytesused + : buf.bytesused; + + for (auto [i, plane] : utils::enumerate(buffer->planes())) { + if (!bytesused) { + LOG(V4L2, Error) + << "Dequeued buffer is too small"; + + metadata.status = FrameMetadata::FrameError; + return buffer; + } + + metadata.planes.push_back({ std::min(plane.length, bytesused) }); + bytesused -= metadata.planes.back().bytesused; + } + } else if (multiPlanar) { + /* + * If we use the multi-planar API, fill in the planes. + * The number of planes in the frame buffer and in the + * V4L2 buffer is guaranteed to be equal at this point. + */ + for (unsigned int i = 0; i < numV4l2Planes; ++i) + metadata.planes.push_back({ planes[i].bytesused }); } else { - buffer->metadata_.planes.push_back({ buf.bytesused }); + metadata.planes.push_back({ buf.bytesused }); } return buffer;
When dequeueing a buffer from a V4L2VideoDevice, the number of planes in the FrameBuffer may not match the number of V4L2 buffer planes if the PixelFormat is multi-planar (has multiple colour planes) and the V4L2 format is single-planar (has a single buffer plane). In this case, we need to split the single V4L2 buffer plane into FrameBuffer planes. Do so, and add checks to reject invalid V4L2 buffers in case of a driver issue. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes since v1: - Reduce indentation --- src/libcamera/v4l2_videodevice.cpp | 53 +++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 5 deletions(-)