| Message ID | 20240909053425.3075699-1-chenghaoyang@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi On Mon, Sep 09, 2024 at 05:34:19AM GMT, Harvey Yang wrote: > From: Han-Lin Chen <hanlinchen@chromium.org> > > To solve issues when platforms not following V4L2 spec and frame > buffers not going through V4L2VideoDevice, setting default values > to FrameMetadata's member variables. I still think this patch works around a faulty implementation, however initializing class members to sane defaults certainly doesn't hurt. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > --- > include/libcamera/framebuffer.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h > index ff839243..8dae747c 100644 > --- a/include/libcamera/framebuffer.h > +++ b/include/libcamera/framebuffer.h > @@ -32,9 +32,9 @@ struct FrameMetadata { > unsigned int bytesused; > }; > > - Status status; > - unsigned int sequence; > - uint64_t timestamp; > + Status status = FrameSuccess; I wonder if this shouldn't be the other way around, make it Fail by default to make sure the component that handles the buffers correctly set their status. > + unsigned int sequence = 0; > + uint64_t timestamp = 0; These ones are indeed sane > > Span<Plane> planes() { return planes_; } > Span<const Plane> planes() const { return planes_; } > -- > 2.46.0.469.g59c65b2a67-goog >
Thanks Jacopo, On Mon, Sep 9, 2024 at 3:33 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi > > On Mon, Sep 09, 2024 at 05:34:19AM GMT, Harvey Yang wrote: > > From: Han-Lin Chen <hanlinchen@chromium.org> > > > > To solve issues when platforms not following V4L2 spec and frame > > buffers not going through V4L2VideoDevice, setting default values > > to FrameMetadata's member variables. > > I still think this patch works around a faulty implementation, however > initializing class members to sane defaults certainly doesn't hurt. > > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > > --- > > include/libcamera/framebuffer.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/libcamera/framebuffer.h > b/include/libcamera/framebuffer.h > > index ff839243..8dae747c 100644 > > --- a/include/libcamera/framebuffer.h > > +++ b/include/libcamera/framebuffer.h > > @@ -32,9 +32,9 @@ struct FrameMetadata { > > unsigned int bytesused; > > }; > > > > - Status status; > > - unsigned int sequence; > > - uint64_t timestamp; > > + Status status = FrameSuccess; > > I wonder if this shouldn't be the other way around, make it Fail by > default to make sure the component that handles the buffers correctly > set their status. > Yes, that makes more sense actually. Setting it to FrameError by default instead in the next version. > > > + unsigned int sequence = 0; > > + uint64_t timestamp = 0; > > These ones are indeed sane > > > > > Span<Plane> planes() { return planes_; } > > Span<const Plane> planes() const { return planes_; } > > -- > > 2.46.0.469.g59c65b2a67-goog > > >
diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index ff839243..8dae747c 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -32,9 +32,9 @@ struct FrameMetadata { unsigned int bytesused; }; - Status status; - unsigned int sequence; - uint64_t timestamp; + Status status = FrameSuccess; + unsigned int sequence = 0; + uint64_t timestamp = 0; Span<Plane> planes() { return planes_; } Span<const Plane> planes() const { return planes_; }