[RFC,v1,24/27] v4l2: v4l2_camera: Use buffer cookie for indexing

26987 diff mbox series

Use the buffer cookie to store the index of the buffer instead of
relying on the 1-to-1 association between requests and buffers,
which will be broken by the split of requests and buffers.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Hi Barnabás

On Thu, Jun 18, 2026 at 02:38:41PM +0200, Barnabás Pőcze wrote:
> Use the buffer cookie to store the index of the buffer instead of
> relying on the 1-to-1 association between requests and buffers,
> which will be broken by the split of requests and buffers.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 75428b8adc..7dd0ea1ef9 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -87,7 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>  	/* We only have one stream at the moment. */
>  	bufferLock_.lock();
>  	FrameBuffer *buffer = request->buffers().begin()->second;
> -	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
> +	completedBuffers_.emplace_back(buffer->cookie(), buffer->metadata());
>  	bufferLock_.unlock();
>
>  	uint64_t data = 1;
> @@ -171,6 +171,8 @@ int V4L2Camera::allocBuffers()
>  			return -ENOMEM;
>  		}
>  		requestPool_.push_back(std::move(request));
> +
> +		buffers[i]->setCookie(i);
>  	}
>
>  	return buffers.size();
> --
> 2.54.0
>

On Thu, Jun 18, 2026 at 02:38:41PM +0200, Barnabás Pőcze wrote:
> Use the buffer cookie to store the index of the buffer instead of
> relying on the 1-to-1 association between requests and buffers,
> which will be broken by the split of requests and buffers.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 75428b8adc..7dd0ea1ef9 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -87,7 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>  	/* We only have one stream at the moment. */
>  	bufferLock_.lock();
>  	FrameBuffer *buffer = request->buffers().begin()->second;
> -	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
> +	completedBuffers_.emplace_back(buffer->cookie(), buffer->metadata());
>  	bufferLock_.unlock();
>  
>  	uint64_t data = 1;
> @@ -171,6 +171,8 @@ int V4L2Camera::allocBuffers()

Would it be more efficient to iterate over buffers here ?

	for (auto [i, buffer] : utils::enumerate(buffers)) {
		...

		buffer->setCookie(i);
	}

?

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

>  			return -ENOMEM;
>  		}
>  		requestPool_.push_back(std::move(request));
> +
> +		buffers[i]->setCookie(i);
>  	}
>  
>  	return buffers.size();

2026. 06. 24. 0:54 keltezéssel, Laurent Pinchart írta:
> On Thu, Jun 18, 2026 at 02:38:41PM +0200, Barnabás Pőcze wrote:
>> Use the buffer cookie to store the index of the buffer instead of
>> relying on the 1-to-1 association between requests and buffers,
>> which will be broken by the split of requests and buffers.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   src/v4l2/v4l2_camera.cpp | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
>> index 75428b8adc..7dd0ea1ef9 100644
>> --- a/src/v4l2/v4l2_camera.cpp
>> +++ b/src/v4l2/v4l2_camera.cpp
>> @@ -87,7 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>>   	/* We only have one stream at the moment. */
>>   	bufferLock_.lock();
>>   	FrameBuffer *buffer = request->buffers().begin()->second;
>> -	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
>> +	completedBuffers_.emplace_back(buffer->cookie(), buffer->metadata());
>>   	bufferLock_.unlock();
>>
>>   	uint64_t data = 1;
>> @@ -171,6 +171,8 @@ int V4L2Camera::allocBuffers()
> 
> Would it be more efficient to iterate over buffers here ?
> 
> 	for (auto [i, buffer] : utils::enumerate(buffers)) {
> 		...
> 
> 		buffer->setCookie(i);
> 	}
> 
> ?

I suppose, yes, but if you don't like the previous patch, then there will
be a separate "count" variable for the number of requests, so this cannot
be done.


> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>>   			return -ENOMEM;
>>   		}
>>   		requestPool_.push_back(std::move(request));
>> +
>> +		buffers[i]->setCookie(i);
>>   	}
>>
>>   	return buffers.size();
> 
> --
> Regards,
> 
> Laurent Pinchart