[libcamera-devel] [PATCH 2/2] libcamera: v4l2_videodevice: Update bytesused when dq'ing MPLANE buffers

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Oct 18, 2019 at 04:42:01PM +0100, Kieran Bingham wrote:
> When we dequeue a buffer using the MPLANE API, we currently set our
> buffer->bytesused field to zero due to the zero entry in the V4L2 buffer
> bytesused field.
> 
> The bytesused field is only really tracked for debug purposes currently,
> so store the total bytese used from each plane in our local statistics
> to identify actual data flow.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> ---
> 
> This patch comes with a pinch of salt, we could also add a bytesused
> field to the Plane class and intead update all the debug prints to
> report the bytesused on a per-plane basis.

My initial opinion is that handling this in the Buffer and Plane classes
would be better. I think the V4L2-related classes should stay as close
as possible to the V4L2 API, otherwise it could soon become quite
confusing.

Niklas is planning to rework the buffer API this week, he should take
this into consideration.

> ---
>  src/libcamera/v4l2_videodevice.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 37f61b90ac46..71f141e7b511 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1124,13 +1124,20 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
>  		fdEvent_->setEnabled(false);
>  
>  	buffer->index_ = buf.index;
> -	buffer->bytesused_ = buf.bytesused;
>  	buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL
>  			   + buf.timestamp.tv_usec * 1000ULL;
>  	buffer->sequence_ = buf.sequence;
>  	buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR
>  			? Buffer::BufferError : Buffer::BufferSuccess;
>  
> +	if (multiPlanar_) {
> +		buffer->bytesused_ = 0;
> +		for (unsigned int p = 0; p < buf.length; ++p)
> +			buffer->bytesused_ += planes[p].bytesused;
> +	} else {
> +		buffer->bytesused_ = buf.bytesused;
> +	}
> +
>  	return buffer;
>  }
>  

-- 
Regards,

Laurent Pinchart