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

[Niklas Söderlund]

On 2019-10-21 11:36:51 +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 20/10/2019 06:20, Laurent Pinchart wrote:
> > 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.
> 
> That's fine, this patch was mostly sent to highlight the issue, and
> discuss, as noted by the 'pinch of salt'.
> 
> 
> I'll await the buffer rework.

This is addressed in the buffer rework series.

> 
> 
> >> ---
> >>  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
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund