[libcamera-devel] [PATCH v2 14/27] libcamera: v4l2_videodevice: Split planes when dequeuing buffer

[Jean-Michel Hautbois]

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 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 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;
>