[libcamera-devel] [PATCH v2 5/7] v4l2: v4l2_camera_proxy: Don't return -EINVAL for zero sizeimage in REQBUFS

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jun 05, 2020 at 06:01:04PM +0900, Paul Elder wrote:
> If VIDIOC_REQBUFS returns -EINVAL, it signals to the application that
> the requested buffer or memory type is not supported. If we return
> -EINVAL due to a zero sizeimage, then the application will think that we
> don't support a memory type that we actually do. We cannot error on a
> zero sizeimage, because reqbufs could be called merely to probe what IO
> methods we support; qv4l2, for example, called reqbufs once with userptr
> and once more with mmap, both times with count=1.
> 
> On the other hand, sizeimage will be zero for formats whose size we
> don't know how to calculate, such as MJPEG. If we try to stream such
> formats anyway, we will get a floating point exception and crash. Issue
> a warning for now, and don't return -EINVAL, so that we can continue
> operation.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
> Changes in v2: expand changelog
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index a0c6deea..cbe9e026 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -352,8 +352,18 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)
>  		return -EINVAL;
>  
>  	sizeimage_ = calculateSizeImage(streamConfig_);
> +	/*
> +	 * If we return -EINVAL here then the application will think that we
> +	 * don't support streaming mmap. Since we don't support readwrite and
> +	 * userptr either, the application will get confused and think that
> +	 * we don't support anything.
> +	 * On the other hand, if a format has a zero sizeimage (eg. MJPEG),
> +	 * we'll get a floating point exception when we try to stream it.
> +	 */
>  	if (sizeimage_ == 0)
> -		return -EINVAL;
> +		LOG(V4L2Compat, Warning)
> +			<< "sizeimage of at least one format is zero. "
> +			<< "Streaming this format will cause a floating point exception.";
>  
>  	setFmtFromConfig(streamConfig_);
>  

-- 
Regards,

Laurent Pinchart