[PATCH] V4L2VideoDevice: Call FrameBuffer::Private::cancel() in streamOff()

[Cheng-Hao Yang]

Hi Kieran,

On Mon, Dec 9, 2024 at 11:01 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Harvey Yang (2024-12-03 09:52:13)
> > 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 at chromium.org>
> > ---
> >  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..0558434cb 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();
> > +               buffer->_d()->cancel();
>
>
> buffer->_d()->cancel() indeed implements:
>
>         void cancel() { metadata_.status = FrameMetadata::FrameCancelled; }
>
> so this looks good to me.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> >
> >                 cache_->put(it.first);
> > -               metadata.status = FrameMetadata::FrameCancelled;
>
> But you have changed "/where/" this is being done. Now the
> metadata.status is being set /before/ cache_->put(it.first); which isn't
> mentioned in the commit message. Is it an intentional change?
>
> I believe it's fine though, but would have been note worthy to make sure
> it was clear you had considered this and it's ok.

Right, I should set status after `cache_->put(it.first);` to avoid
unnecessary changes. Let me upload another version.

BR,
Harvey

>
> --
> Kieran
>
>
> >                 bufferReady.emit(buffer);
> >         }
> >
> > --
> > 2.47.0.338.g60cca15819-goog
> >