[libcamera-devel] [PATCH v3 14/30] libcamera: v4l2_videodevice: Split planes when dequeuing buffer
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Sep 7 11:40:22 CEST 2021
On 07/09/2021 01:05, Laurent Pinchart wrote:
> Hi Kieran,
>
> On Tue, Sep 07, 2021 at 12:54:15AM +0100, Kieran Bingham wrote:
>> On 06/09/2021 23:56, 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.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at 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 6c1ca9bb0a5c..2e458fcc22e0 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>
>>> @@ -1669,12 +1670,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()) {
>
> *1
>
>>> + /*
>>> + * 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() << ")";
>>
>> I'm weary that this error print is more about the first conditional
>> statement, but I think it still fits here anyway.
>
> Did you mean *1 ? I'm not sure to follow you.
Yes, the error print matches the conditional check of *1, but the actual
error is that
(numV4l2Planes != buffer->planes().size)
&& numV4l2Planes != 1.
It will be evident enough what has happened to any reader digging into
that error message, so it's fine.
>
>>> +
>>> + 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 was going to ask about the metadata plan allocations, but now I see
>> that's handled in 16/30
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>> }
>>>
>>> return buffer;
>
More information about the libcamera-devel
mailing list