[libcamera-devel] [RFC PATCH 10/10] libcamera: framebuffer: Add assertion to detect offset is unfilled
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Aug 20 17:31:24 CEST 2021
Hi Hiro,
On Fri, Aug 20, 2021 at 08:18:47PM +0900, Hirokazu Honda wrote:
> On Wed, Aug 18, 2021 at 10:42 AM Laurent Pinchart wrote:
> > On Mon, Aug 16, 2021 at 01:31:38PM +0900, Hirokazu Honda wrote:
> > > The offset variable is introduced to FrameBuffer::Plane. In order to
> > > detect that the plane is used while the offset is not set, this adds
> > > the assertion to FrameBuffer::planes(). It should be removed in the
> > > future.
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > > ---
> > > include/libcamera/framebuffer.h | 13 +++++++++++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
> > > index 5de3c744..79541557 100644
> > > --- a/include/libcamera/framebuffer.h
> > > +++ b/include/libcamera/framebuffer.h
> > > @@ -7,6 +7,8 @@
> > > #ifndef __LIBCAMERA_FRAMEBUFFER_H__
> > > #define __LIBCAMERA_FRAMEBUFFER_H__
> > >
> > > +#include <assert.h>
> > > +#include <limits>
> > > #include <stdint.h>
> > > #include <vector>
> > >
> > > @@ -41,14 +43,21 @@ class FrameBuffer final : public Extensible
> > >
> > > public:
> > > struct Plane {
> > > + static constexpr unsigned int kInvalidOffset = std::numeric_limits<unsigned int>::max();
> >
> > Maybe a blank line here ?
> >
> > This also generates a doxygen warning as kInvalidOffset is not
> > documented. As it's not meant to be used by anyone else than the code
> > below, this could be addressed by making the constant private.
> >
>
> I couldn't move it to private because it is used in the below assert.
> I document the variable.
>
> > > FileDescriptor fd;
> > > - unsigned int offset;
> > > + unsigned int offset = kInvalidOffset;
> > > unsigned int length;
> > > };
> > >
> > > FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0);
> > >
> > > - const std::vector<Plane> &planes() const { return planes_; }
> > > + const std::vector<Plane> &planes() const
> > > + {
> > > + /* \todo remove this assert. */
> >
> > When do you foresee this being removed ?
>
> Maybe for a month. Just an arbitrary foresee.
Then let's write
/* \todo Remove the assertions after sufficient testing */
> > > + for (const auto &plane : planes_)
> > > + assert(plane.offset != Plane::kInvalidOffset);
> > > + return planes_;
> > > + }
> > >
> > > Request *request() const;
> > > const FrameMetadata &metadata() const { return metadata_; }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list