[libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Improve debugging when buffer is too small

[Kieran Bingham]

Quoting Laurent Pinchart (2021-10-08 11:15:56)
> When a dequeued buffer is too small, the condition is logged and an
> error is returned. The logged message doesn't provide any information
> about the sizes, making debugging more difficult. Improve it by logging
> both the bytesused value and the length of each plane.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Compile-tested only. Eugen, could you give it a try to debug the problem
> you're facing ?
> 
> ---
>  src/libcamera/v4l2_videodevice.cpp | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index ba5f88cd41ed..bd74103de7af 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1713,19 +1713,25 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
>  
>                 unsigned int bytesused = multiPlanar ? planes[0].bytesused
>                                        : buf.bytesused;
> +               unsigned int remaining = bytesused;
>  
>                 for (auto [i, plane] : utils::enumerate(buffer->planes())) {
> -                       if (!bytesused) {
> +                       if (!remaining) {
>                                 LOG(V4L2, Error)
> -                                       << "Dequeued buffer is too small";
> +                                       << "Dequeued buffer (" << bytesused
> +                                       << " bytes) too small for plane lengths "
> +                                       << utils::join(buffer->planes(), "/",
> +                                                      [](const FrameBuffer::Plane &p){
> +                                                              return p.length;
> +                                                      });
>  
>                                 metadata.status = FrameMetadata::FrameError;
>                                 return buffer;
>                         }
>  
>                         metadata.planes()[i].bytesused =
> -                               std::min(plane.length, bytesused);
> -                       bytesused -= metadata.planes()[i].bytesused;
> +                               std::min(plane.length, remaining);
> +                       remaining -= metadata.planes()[i].bytesused;

Should this prevent underflowing 'remaining' - or otherwise, make
remaining a signed int, and check against if (remaining <= 0) above
instead.

I'd prefer in this context to use the signed value and make sure we
clearly catch negative remaining..

With that,


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>                 }
>         } else if (multiPlanar) {
>                 /*
> 
> base-commit: 962df634bd0afe12e6f38464f5e602cf1460c430
> -- 
> Regards,
> 
> Laurent Pinchart
>