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

[Kieran Bingham]

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.


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