[v2] V4L2VideoDevice: Call FrameBuffer::Private::cancel() in streamOff()
diff mbox series

Message ID 20241209163418.330904-1-chenghaoyang@chromium.org
State New
Headers show
Series
  • [v2] V4L2VideoDevice: Call FrameBuffer::Private::cancel() in streamOff()
Related show

Commit Message

Cheng-Hao Yang Dec. 9, 2024, 4:34 p.m. UTC 
At the moment `V4L2VideoDevice::streamOff()` sets
`FrameBuffer::Private`'s metadata directly, while that's equivalent to
calling `FrameBuffer::Private::cancel()`. To ease code tracing, this
patch replace the manual modification with the function call.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/v4l2_videodevice.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Kieran Bingham June 27, 2025, 8:17 a.m. UTC | #1 
Hi all,

Quoting Harvey Yang (2024-12-09 16:34:07)
> At the moment `V4L2VideoDevice::streamOff()` sets
> `FrameBuffer::Private`'s metadata directly, while that's equivalent to
> calling `FrameBuffer::Private::cancel()`. To ease code tracing, this
> patch replace the manual modification with the function call.
> 
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

I'm digging through my emails and see this is still pending.

I believe it's suitable for merge, but it's core libcamera and only has
one RB tag from me.

any other feedback or otherwise objections to this? Otherwise if no one
objects I'll merge.

--
Kieran


> ---
>  src/libcamera/v4l2_videodevice.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index a5cf67845..5fcebdc63 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -2007,10 +2007,9 @@ int V4L2VideoDevice::streamOff()
>         /* Send back all queued buffers. */
>         for (auto it : queuedBuffers_) {
>                 FrameBuffer *buffer = it.second;
> -               FrameMetadata &metadata = buffer->_d()->metadata();
>  
>                 cache_->put(it.first);
> -               metadata.status = FrameMetadata::FrameCancelled;
> +               buffer->_d()->cancel();
>                 bufferReady.emit(buffer);
>         }
>  
> -- 
> 2.47.0.338.g60cca15819-goog
>
Umang Jain June 27, 2025, 8:25 a.m. UTC | #2 
Hello,

On 6/27/25 1:47 PM, Kieran Bingham wrote:
> Hi all,
>
> Quoting Harvey Yang (2024-12-09 16:34:07)
>> At the moment `V4L2VideoDevice::streamOff()` sets
>> `FrameBuffer::Private`'s metadata directly, while that's equivalent to


nit: s/metadata/metadata status to FrameCancelled/

>> calling `FrameBuffer::Private::cancel()`. To ease code tracing, this
>> patch replace the manual modification with the function call.
>>
>> Signed-off-by: Harvey Yang<chenghaoyang@chromium.org>
>> Reviewed-by: Kieran Bingham<kieran.bingham@ideasonboard.com>


LGTM,

Reviewed-by: Umang Jain<uajain@igalia.com>


> I'm digging through my emails and see this is still pending.
>
> I believe it's suitable for merge, but it's core libcamera and only has
> one RB tag from me.
>
> any other feedback or otherwise objections to this? Otherwise if no one
> objects I'll merge.
>
> --
> Kieran
>
>
>> ---
>>   src/libcamera/v4l2_videodevice.cpp | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
>> index a5cf67845..5fcebdc63 100644
>> --- a/src/libcamera/v4l2_videodevice.cpp
>> +++ b/src/libcamera/v4l2_videodevice.cpp
>> @@ -2007,10 +2007,9 @@ int V4L2VideoDevice::streamOff()
>>          /* Send back all queued buffers. */
>>          for (auto it : queuedBuffers_) {
>>                  FrameBuffer *buffer = it.second;
>> -               FrameMetadata &metadata = buffer->_d()->metadata();
>>   
>>                  cache_->put(it.first);
>> -               metadata.status = FrameMetadata::FrameCancelled;
>> +               buffer->_d()->cancel();
>>                  bufferReady.emit(buffer);
>>          }
>>   
>> -- 
>> 2.47.0.338.g60cca15819-goog
>>

Patch
diff mbox series 

diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index a5cf67845..5fcebdc63 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -2007,10 +2007,9 @@  int V4L2VideoDevice::streamOff()
 	/* Send back all queued buffers. */
 	for (auto it : queuedBuffers_) {
 		FrameBuffer *buffer = it.second;
-		FrameMetadata &metadata = buffer->_d()->metadata();
 
 		cache_->put(it.first);
-		metadata.status = FrameMetadata::FrameCancelled;
+		buffer->_d()->cancel();
 		bufferReady.emit(buffer);
 	}