[PATCH] libcamera: Make FrameBuffer status default to FrameSuccess

[Han-lin Chen]

Hi Jacopo,

On Tue, Aug 27, 2024 at 6:06 PM Jacopo Mondi <jacopo.mondi at ideasonboard.com>
wrote:

> Hi Han-Lin
>
> On Mon, Aug 26, 2024 at 01:46:30PM GMT, Harvey Yang wrote:
> > From: Han-Lin Chen <hanlinchen at chromium.org>
> >
> > There seems to be an assumption that a FrameBuffer is success unless
> > the pipeline handler canceled the frame, or there is a failure
> > processing the FrameBuffer.
>
> I'm not sure this is actually assumed anywhere.
>
> The FrameMetadata status is set by the V4L2VideoDevice class at
> dequeue buffer time
>
> src/libcamera/v4l2_videodevice.cpp-     metadata.status = buf.flags &
> V4L2_BUF_FLAG_ERROR
> src/libcamera/v4l2_videodevice.cpp-                     ?
> FrameMetadata::FrameError
> src/libcamera/v4l2_videodevice.cpp:                     :
> FrameMetadata::FrameSuccess;
>

Thanks!
The patch was a temporary workaround in our branch, and accidentally copied
here. Sorry for the confusion.
There was a time when the platform was not following the V4L2 spec and some
buffers may not go through the V4L2VideoDevice class, and the status
remains undefined, causing some problems during development.

>
> > Make the assumption specific.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > ---
> >  include/libcamera/framebuffer.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/framebuffer.h
> b/include/libcamera/framebuffer.h
> > index 5ae2270b3..71202a1d2 100644
> > --- a/include/libcamera/framebuffer.h
> > +++ b/include/libcamera/framebuffer.h
> > @@ -33,7 +33,7 @@ struct FrameMetadata {
> >               unsigned int bytesused;
> >       };
> >
> > -     Status status;
> > +     Status status = FrameSuccess;
> >       unsigned int sequence;
> >       uint64_t timestamp;
>
> However, I don't think this hurts but I wonder why sequence or
> timestamp are any different.
>

We should re-format the patch to simply add default values to all fields,
with a clearer description.

>
> >
> > --
> > 2.46.0.295.g3b9ea8a38a-goog
> >
>


-- 
Cheers.
Hanlin Chen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240827/ef972c5b/attachment.htm>